improvement(admin): update defaults for better UX - #6112
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Credential creation now plans workspace context inside the transaction, acquires those locks, re-validates write access (403/409 if org or permissions changed), and seeds memberships from the locked context. Personal env credential sync runs in one transaction behind the same user-identity fence with chunked inserts/deletes. Workspace creation replaces ad-hoc billing-identity checks with Admin workspace moves accept optional Smaller changes: Reviewed by Cursor Bugbot for commit 1f26f7f. Configure here. |
Greptile SummaryThis PR strengthens transactional consistency across administrative workspace, invitation, credential, billing, and cleanup operations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains in the eligible follow-up review scope.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/invitations/core.ts | Centralizes invitation acceptance, updates, revocation, authorization, and concurrency control into transactional mutation flows. |
| apps/sim/lib/workspaces/admin-move.ts | Expands administrative workspace moves to coordinate ownership validation, invitation migration, storage billing, permissions, credentials, and outbox work. |
| apps/sim/app/api/credentials/route.ts | Re-authorizes credential creation under shared organization and user mutation locks before inserting credentials and memberships. |
| apps/sim/lib/billing/organizations/membership.ts | Adds shared organization-user locking and coordinates membership mutations with invitation and workspace state. |
| apps/sim/lib/cleanup/batch-delete.ts | Introduces reusable bounded batch-selection and deletion machinery used by the revised cleanup jobs. |
| apps/sim/lib/core/outbox/service.ts | Updates outbox processing and coalescing behavior to support deferred administrative and invitation side effects. |
Sequence Diagram
sequenceDiagram
participant Admin as Admin/API caller
participant Service as Transactional service
participant Locks as Advisory and row locks
participant DB as PostgreSQL
participant Outbox as Outbox worker
Admin->>Service: Move workspace or mutate invitation
Service->>Locks: Acquire invitation/workspace/org locks
Locks-->>Service: Mutation scope serialized
Service->>DB: Re-read authoritative state
Service->>Service: Re-authorize and validate expected state
alt State remains valid
Service->>DB: Apply coordinated mutations
Service->>Outbox: Enqueue post-commit work
DB-->>Admin: Commit result
else State changed or authorization lost
Service-->>Admin: Conflict or forbidden response
end
Reviews (2): Last reviewed commit: "refactor: address review nits on the adm..." | Re-trigger Greptile
Correct the attach lock-order comment. The order matches admin move, and what makes it mandatory is invitation acceptance: it holds `workspace-invitations:<id>` while waiting for the workspace row, so row-locking first (as this did) deadlocks against it. The previous ownership-transfer justification did not hold — that path takes the organization lock before its workspace rows too, so the two agree on order rather than inverting. Drop `cancelInvitation`. The `revokeInvitationAsAdmin` extraction left it with no callers, and an unlocked, unauthorized `status = 'cancelled'` flip sitting next to the fenced replacement is easy to reach for by mistake. Drop the unused `executor` parameters from `hasWorkspaceAdminAccess` and `isOrganizationAdminOrOwner`. No caller threads a transaction through either, and the former goes back to delegating to `checkWorkspaceAccess` instead of re-deriving the same permission itself. Import `chunkArray` from `@sim/utils/helpers` everywhere and remove the re-export from `batch-delete.ts`, so the symbol has one source rather than a non-barrel shim plus the package. Restore the bounded attachability check in `addDashboardOrganizationMember`: scope the query to the selected ids instead of listing every attachable workspace and scanning that array per selection. Resolve the credential-creation permission through `getEffectiveWorkspacePermission` rather than a second copy of the org-admin derivation ladder, so the rule cannot drift from the shared resolver. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run |
`revokeInvitationWorkspaceGrant` lost its only caller when the DELETE route moved to `revokeInvitationAsAdmin`, leaving a locked wrapper nothing invoked. Remove it and fold its documentation into `revokeInvitationWorkspaceGrantTx`, which direct grants and scoped revocation still call. The grant-revocation test now drives the transactional form directly, so the sibling-grant and final-grant-cancels behaviour it covers stays under test. `isSameOrgMember` has had no caller since before this branch — direct grant resolves membership through `getUserOrganization` inside its own transaction — so it and its tests go too. `getWorkspaceMembership` is no longer imported outside its module now that credential creation reads `getCredentialCreationWorkspaceContext`; make it module-private rather than leave it on the public surface. Co-Authored-By: Claude <noreply@anthropic.com>
Each was checked by hand across every file type, including barrel re-exports and string references, rather than taken from a static analyzer. `isUserMemberOfOrganization` has no reference anywhere. `reapplyPaidOrgJoinBillingForExistingMember` only ever ran from two lock-ordering tests. The transaction-enlisted form it delegated to is what the subscription webhooks call and what those tests actually assert on, so they now drive it directly. The assertions are unchanged: the wrapper contributed a transaction, an organization lock and a membership existence check, none of which appear in the recorded operations. `replaceLargeValueReferences` and `replaceLargeValueReferencesWithClient` are both thin wrappers over `replaceLargeValueReferenceKeysWithClient`, which execution logging, human-in-the-loop resume and the trace backfill all still call. The single test covering a wrapper now composes the key collection itself and targets that live helper. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1f26f7f. Configure here.
Summary
Admin routes dependent changes + perf improvements pass
Type of Change
Testing
N/A
Checklist